-
Notifications
You must be signed in to change notification settings - Fork 455
Add from/to timestamp filter to get blocks endpoint - Closes #2391 #2472
Add from/to timestamp filter to get blocks endpoint - Closes #2391 #2472
Conversation
@@ -106,6 +106,16 @@ __private.list = function(filter, cb) { | |||
params.height = filter.height; | |||
} | |||
|
|||
if (filter.fromTimestamp >= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In swagger we say 0
is the valid value. Should not we simplify it on API level to disallow zero as param value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to keep 0
as an allowed value. However, there was a mistake in the control of toTimestamp
that I've caught thanks to this comment. @nazarhussain could you check it again? I hope now it's easier to understand.
modules/blocks/api.js
Outdated
params.fromTimestamp = filter.fromTimestamp; | ||
} | ||
|
||
if (filter.toTimestamp >= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
test/functional/http/get/blocks.js
Outdated
@@ -112,6 +113,72 @@ describe('GET /blocks', () => { | |||
}); | |||
}); | |||
|
|||
describe('fromTimestamp', () => { | |||
it('using too small fromTimestamp should fail', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using invalid timestamp should fail
?
test/functional/http/get/blocks.js
Outdated
}); | ||
|
||
describe('toTimestamp', () => { | ||
it('using too small toTimestamp should fail', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may add validation to check if fromTimestamp
is always less than toTimestamp
. Its not that much needed but may be useful.
…o_timestamp_filter_get_blocks
How to test it?
mocha test/functional/http/get/blocks.js
Review checklist